Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements several significant updates to the cloud-native bootstrapper: adds AWS session token support for temporary credentials, removes telemetry functionality entirely, and introduces support for using pre-existing Kubernetes secrets for Mattermost workspace creation.
- Adds AWS session token support throughout the authentication flow and credential management
- Completely removes telemetry infrastructure including APIs, hooks, and backend components
- Adds capability to use existing Kubernetes secrets for database and filestore configurations
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| webapp/src/types/bootstrapper.ts | Adds sessionToken field to CloudCredentials type |
| webapp/src/types/Installation.ts | Adds optional secret field to S3FileStore interface |
| webapp/src/store/installation/bootstrapperSlice.ts | Updates initialState to include empty sessionToken |
| webapp/src/store/index.ts | Removes telemetryApi from store configuration |
| webapp/src/pages/setup/setup.scss | Adds styling for session info display |
| webapp/src/pages/setup/index.tsx | Implements server-side session checking and UI updates |
| webapp/src/pages/setup/get_credentials.tsx | Adds session token input fields |
| webapp/src/pages/mattermost/filestore_connection.tsx | Adds existing secret toggle functionality |
| webapp/src/pages/mattermost/db_connection.tsx | Adds existing secret support for database connections |
| webapp/src/pages/mattermost/create_workspace.tsx | Updates validation logic for secret-based configurations |
| webapp/src/pages/aws/choose_existing.tsx | Adds refetch functionality and improves region selection |
| telemetry/telemetry.go | Removes entire telemetry package |
| providers/aws.go | Integrates session token support throughout AWS authentication |
| model/mattermost.go | Adds secret name fields and validation logic |
| model/cluster.go | Adds SessionToken field to Credentials struct |
| api/state.go | Adds session check endpoint and state update functions |
| api/context.go | Removes telemetry provider from context |
| api/bootstrapper.go | Updates credential handling and Mattermost operator configuration |
| api/api.go | Removes telemetry API initialization |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| type Props = { | ||
| cloudProvider: string; | ||
| kubernetesOption: string; | ||
| onCredentialsChange: (credentials: { accessKeyId: string, accessKeySecret: string, kubeconfig: string }) => void; | ||
| onCredentialsChange: (credentials: { accessKeyId: string, accessKeySecret: string, sessionToken: string, kubeconfig: string }) => void; | ||
| }; | ||
|
|
||
| function GetCredentials({ cloudProvider, kubernetesOption, onCredentialsChange }: Props) { | ||
| const [credentials, setCredentials] = useState({ |
There was a problem hiding this comment.
The inline type definition is duplicated and inconsistent with the credentials state object which includes kubeconfigType. Consider extracting this to a shared type or interface to maintain consistency.
| const filestoreConnectionComplete = filestoreComplete(); | ||
|
|
||
| if (dbConnection.dbConnectionOption === 'Existing' && !!dbConnection.existingDatabaseConfig?.dbConnectionString && !!dbConnection.existingDatabaseConfig?.dbReplicasConnectionString) { | ||
| if (dbConnection.dbConnectionOption === 'Existing' && (dbConnection.existingDatabaseSecretName || (!!dbConnection.existingDatabaseConfig?.dbConnectionString && !!dbConnection.existingDatabaseConfig?.dbReplicasConnectionString))) { |
There was a problem hiding this comment.
This complex boolean logic is difficult to read and maintain. Consider extracting this validation into a separate function like isDatabaseConnectionComplete() for better readability.
| @@ -1219,11 +1246,11 @@ func handleDeployMattermostOperator(c *Context, w http.ResponseWriter, r *http.R | |||
| } | |||
There was a problem hiding this comment.
The commented-out version should either be removed if no longer needed, or the comment should explain why it's commented out and under what conditions it should be used.
| } | |
| UpgradeCRDs: true, | |
| // To pin the chart to a specific version, uncomment the line below and set the desired version. |
Summary
Updates:
Screenshots
Ticket Link
Release Note